-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Config file support #326
base: main
Are you sure you want to change the base?
Config file support #326
Conversation
|
611e678
to
257c433
Compare
81fb36f
to
e9d2621
Compare
3b245e0
to
5365c0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a pretty good start to me. I've left mostly nitpicking comments in-line. 🙂
Makefile.am
Outdated
@@ -17,6 +17,7 @@ AM_CPPFLAGS = $(LIBFIDO2_CFLAGS) $(LIBCRYPTO_CFLAGS) | |||
if ENABLE_FUZZING | |||
AM_CPPFLAGS += -fsanitize=fuzzer-no-link | |||
endif | |||
AM_CPPFLAGS += -D SYSCONFDIR='"$(sysconfdir)"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to make it possible to choose the entire path to a directory rather than just a prefix (see CFG_DEFAULT_PATH
) ? E.g.
--with-security-config-dir=path
where path defaults to $sysconfdir/security at configure-time.
Having it into another module will prevent the code from being messy later. The parsing procedure is taken verbatim: no semantic change, no behavioural change.
5365c0d
to
98fdb4d
Compare
Current situation:What is left to do for the current PR (soon follow-up):
|
0adeda2
to
a6e93f3
Compare
Oh no. I figured out this even more work! Autoconf BUG (?)
This is a little pathological (maybe low prio?) but actually a defect. Vulnerable for untidy sysadmin:Currently my code is checking that the configuration file (1) belongs to root, (2) can only be written by root, (3) is not a symlink. But I'm not checking the status of the above directories... Therefore, assuming that
[1] world-writable directory Attack:
|
The configuration file defines the default behaviour of pam_u2f. Individual module invocations under /etc/pam.d can override settings. The file-system location of the config file is by default $sysconfdir/security/pam_u2f.conf, where $sysconfdir is supplied at build time. A new module configuration, "conf=", allows to override it at runtime. Only absolute paths are accepted.
- split-input format: add trailing blob for config file The corpus needs some update. - wrappers (-Wl,--wrap) integrate fuzzing of the configuration file. The configuration file, mutated by the fuzzer, is made available to the cfg.c implementation. The mock-up works under the assumption that only the cfg.c module works by opening "/" with open(3), and follows up with an alternation of openat(3) and fstat(3) calls.
Generate pam_u2f.8.txt from pam_u2f.8.txt.in, replacing SCONFDIR
a6e93f3
to
36e1255
Compare
Fixes #265